Auto-fixes for jeff/feat/g1_raycast_ivan#2354
Conversation
The color_lookup_table param of OccupancyGrid.to_rerun is a uint8 RGBA LUT; type it precisely per the repo convention. NDArray is already imported under TYPE_CHECKING and the file has future annotations.
This was the only blueprint file with a top-level 'import rerun'. Move it inside _g1_path_colors to match every other blueprint (rerun is a heavy/optional dep and blueprint imports must have no side effects).
The TF parent used self.frame_id (Module property, applies frame_id_prefix) while the child used raw self.config.child_frame_id. With a prefix set, the parent would be namespaced and the child not, breaking the TF tree. The C++ side emits raw config frame ids in message headers (via to_cli_args), so derive both from raw config to match.
paul-nechifor
requested review from
leshy,
mustafab0 and
spomichter
as code owners
Greptile SummaryThis PR applies three automated fixes to the G1 Raycast feature branch, each as a separate commit.
Confidence Score: 5/5All three changes are narrow, mechanical corrections with no new logic introduced. The No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Odom as Odometry stream
participant FastLio2
participant Config as FastLio2Config
participant TF as tf.publish
Odom->>FastLio2: _on_odom_for_tf(msg)
FastLio2->>Config: self.config.frame_id (fixed)
FastLio2->>Config: self.config.child_frame_id
FastLio2->>TF: Transform(frame_id, child_frame_id, ...)
Reviews (2): Last reviewed commit: "fix: derive FastLio2 TF frame ids consis..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| host_ip="192.168.123.164", | ||
| lidar_ip="192.168.123.120", |
There was a problem hiding this comment.
The previous code read os.getenv("LIDAR_HOST_IP", "192.168.123.164") and os.getenv("LIDAR_IP", "192.168.123.120"), allowing deployments on different networks to override the addresses without touching source. Now host_ip and lidar_ip are literal strings, so any machine whose host or LiDAR lives on a different subnet requires a code change. This is a regression if this blueprint is reused across environments or test benches.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| ) | ||
|
|
||
| __all__ = ["unitree_g1_vis"] | ||
| __all__ = ["_unitree_g1_vis"] |
There was a problem hiding this comment.
__all__ is contradictory
__all__ conventionally lists the module's public API — names that consumers are meant to import. Exporting a leading-underscore name (_unitree_g1_vis) through __all__ sends contradictory signals: the underscore marks it as internal, but __all__ marks it as public. Consider either removing __all__ entirely (since the file is a shared sub-blueprint) or documenting why the name is prefixed yet exported.
| __all__ = ["_unitree_g1_vis"] | |
| __all__ = ["_unitree_g1_vis"] # internal sub-blueprint, exported for sibling blueprints |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
paul-nechifor
force-pushed
the
jeff/feat/g1_raycast_ivan-autofixes2
branch
from
340067c to
c220b10
Compare
1 participant
These are automated fixes. Each fix is a separate commit. Use
git rebase -ito drop any you disagree with.